Add support for package.json exports field#904
Conversation
If a module `package.json` specifies `exports`, GraalJs will now read them and prefer exports over standard resolution; export types can be registered by the developer as preferred. In lieu of these types (and as a default), the following export types are preferred, in order: - `graaljs` - `import` (in ESM) - `require` - `default` Fixes and closes oracle#903 Relates-to: oracle#903 Signed-off-by: Sam Gammon <sam@elide.dev>
oracle-contributor-agreement
bot
added
the
OCA Verified
sgammon
changed the title
package.json exports fieldpackage.json exports field.
sgammon
changed the title
package.json exports field.package.json exports field
|
I don't know how to add tests to GraalJs yet. I'm happy to figure it out and add tests if there is any desire to merge this |
| ); | ||
| } | ||
|
|
||
| public static void registerPreferredExportType(String exportType) { |
There was a problem hiding this comment.
This would be used by embedders:
NpmCompatibleESModuleLoader.registerPreferredExportType("elide");... would register the engine elide as a preferred token, greater in precedence than graaljs, import, require, or default. Perhaps a browser builder could register "browser" to prefer imports of that type.
There was a problem hiding this comment.
I see... could you do this in a way that does not require modifying a static final list?
There was a problem hiding this comment.
@woess I can try; for example, what if the embedder registers a callback to provide a list of candidate strings? I just figure the callback would probably not need to be dynamic in nature (these strings are likely registered once at startup and not again).
I could wrap the list in an atomic, but that wouldn't do much. What would you recommend as an approach for this one?
|
Happy to accept contributions in that area. The commonjs support has been a bit neglected tbh... We will need some test coverage for it. I think you can find most of the tests here: |
|
@woess Thanks for the review. I'll take a look at adding some tests |
| // 1.1: is it specified within the exports? | ||
| if (exports.containsKey(preferred)) { | ||
| // 1.2: if so, resolve the import from the package root. make sure to slice off the `./` prefix. | ||
| return packageUrl.resolve(exports.get(preferred).substring(2)); |
There was a problem hiding this comment.
I don't see a check that the string actually starts with ./ or is has length >= 2, only a startsWith(".").
There was a problem hiding this comment.
Good catch, will add
|
This is great, especially Conditional exports support. "exports": {
"./features/*.js": "./src/features/*.js"
},for |
3 participants
If a module
package.jsonspecifiesexports, GraalJs will now read them and prefer exports over standard resolution; export types can be registered by the developer as preferred. In lieu of these types (and as a default), the following export types are preferred, in order:graaljsimport(in ESM)requiredefaultFixes and closes #903
This is quick and dirty and probably not mergeable as expressed, but we are using it effectively downstream.
cc / @woess